Feature/UI hardening - #101
Conversation
…rome, settings toggle - Add cosmic-console.css with starfield/aurora overlay, chamfered panel chrome, Evan pulse CSS - Add cosmic design tokens to game-polish.chrome.tokens.css and level-select.polish.css - Add display.cosmicConsole setting (v3) with URL override (?cosmic=on/off) and settings dialog toggle - Integrate EvanPulse module for target highlighting (Milestone 4 foundation) - Update service worker precache for new assets - Add cosmic-console-toggle.spec.js (5 tests passing) Milestones 1-3 complete. Milestone 4 (Evan pulse integration) needs presenter to pass actual target elements. Milestone 5 (docs/screenshots) pending.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideThis PR hardens the UI by restructuring the welcome and level-selection pages around a shared responsive shell, centralizing settings behavior, adding opt-in Cosmic Console visuals, highlighting Evan helper targets, and extending Playwright coverage for the new interactions and accessibility-oriented states. Sequence diagram for applying shared UI settingssequenceDiagram
actor User
participant Page as WelcomeOrLevelPage
participant Dialog as SettingsDialog
participant Store as UserSettings
participant UI as DocumentUI
User->>Page: Click settings button
Page->>Dialog: mount.open()
Dialog->>Store: getSettings()
Store-->>Dialog: normalized settings
Dialog->>UI: applyUiScale()
Dialog->>UI: applyHighContrast()
Dialog->>UI: applyReducedMotion()
Dialog->>UI: applyCosmicConsole()
User->>Dialog: Change setting
Dialog->>Store: updateSettings(partial, source)
Store-->>Dialog: updated settings
Dialog->>UI: populateForm(updated settings)
Sequence diagram for Evan helper target highlightingsequenceDiagram
participant Runtime as EvanHelperRuntime
participant Presenter as EvanPresenter
participant Pulse as EvanPulse
participant Target as GameplayTarget
Runtime->>Presenter: moveHandTo(x, y, bounds, target)
Presenter->>Pulse: setTarget(target)
Pulse->>Target: add evan-pulse-target class
Presenter->>Presenter: flushHandMove()
Presenter->>Pulse: clearTarget()
Pulse->>Target: remove evan-pulse-target class
State diagram for Cosmic Console visualsstateDiagram-v2
[*] --> Disabled
Disabled --> Enabled: updateSettings({display: {cosmicConsole: true}})
Enabled --> Disabled: updateSettings({display: {cosmicConsole: false}})
Enabled --> ReducedMotion: prefers-reduced-motion
ReducedMotion --> Enabled: motion preference restored
Disabled --> [*]
Enabled --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Deploy Preview for mathmasteralgebra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Hey - I've found 6 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/pages/game.html" line_range="374" />
<code_context>
<!-- Onboarding + Evan helper -->
<script defer src="/src/scripts/startup-preload.js?v=20260420-rotation-cache-bust-1"></script>
<script defer src="/src/scripts/evan-helper.presenter.js"></script>
+ <script defer src="/src/scripts/evan-helper.pulse.js"></script>
<script defer src="/src/scripts/game-tutorial-level.js"></script>
<script defer src="/src/scripts/game-onboarding.controller.js"></script>
</code_context>
<issue_to_address>
**issue (broader_impact):** Evan pulse highlighting is nonfunctional on the game page because `evan-helper.pulse.js` is loaded there but `cosmic-console.css`, which defines `.evan-pulse-target`, is not linked by `game.html`. The target class is added without any visible outline or animation.
**Triggers:** When EvanPresenter passes a target element during gameplay.
**Suggested fix:** Link `cosmic-console.css` from `game.html`, or move the pulse styles into a stylesheet already loaded by the game page.
```suggestion
<link rel="stylesheet" href="/src/styles/css/cosmic-console.css">
<script defer src="/src/scripts/evan-helper.pulse.js"></script>
```
</issue_to_address>
### Comment 2
<location path="src/pages/level-select.html" line_range="240" />
<code_context>
+ Settings
+ </button>
</div>
+ <button class="btn btn--danger" id="reset-progress-btn" type="button">
+ <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" aria-hidden="true">
+ <path d="M3 6h18" />
+ <path d="M19 6l-1.4 14a2 2 0 0 1-2 1.8H8.4a2 2 0 0 1-2-1.8L5 6" />
+ <path d="M10 11v6" />
+ <path d="M14 11v6" />
+ <path d="M9 6V4a2 2 0 0 1 2-2h2a2 2 0 0 1 2 2v2" />
+ </svg>
+ Reset
+ </button>
</footer>
</code_context>
<issue_to_address>
**issue (bug_risk):** The reset-progress control was changed from the `.reset-progress-btn` class to only the `#reset-progress-btn` id, but `level-select-page.progress.js` still queries `.reset-progress-btn`. Its `resetButton` reference is null, so clicking Reset no longer resets progress.
**Triggers:** When the user clicks Reset on the level-select page.
**Suggested fix:** Update the progress script to query `#reset-progress-btn`, or retain the old class on the button.
```suggestion
<button class="btn btn--danger reset-progress-btn" id="reset-progress-btn" type="button">
```
</issue_to_address>
### Comment 3
<location path="src/scripts/level-select-page.settings.js" line_range="13" />
<code_context>
+ const api = window.SettingsDialog.mount({
+ shellId: "level-select-settings-shell",
+ dialogId: "level-select-settings-dialog",
+ openButtonSelector: ".settings-button, #level-select-settings-icon",
+ idPrefix: "",
});
</code_context>
<issue_to_address>
**issue (bug_risk):** The shared settings controller's selector includes `.settings-button` and `#level-select-settings-icon`, but the footer Settings button has only `id="settings-button"` and no `settings-button` class. The controller therefore binds only the top-right icon, leaving the footer Settings button inert.
**Triggers:** When the user clicks the footer Settings button on the level-select page.
**Suggested fix:** Add `#settings-button` to `openButtonSelector`, or restore the `settings-button` class on the footer button.
```suggestion
openButtonSelector: ".settings-button, #level-select-settings-icon, #settings-button",
```
</issue_to_address>
### Comment 4
<location path="src/pages/level-select.html" line_range="39-44" />
<code_context>
- <span class="route-atlas-index">02</span>
- <span class="route-atlas-copy">Switch pace</span>
+ <header class="page-topbar" role="banner">
+ <button class="btn btn--tertiary" id="back-button-top" type="button">
+ <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" aria-hidden="true">
+ <line x1="19" y1="12" x2="5" y2="12" />
+ <polyline points="12 19 5 12 12 5" />
+ </svg>
+ Back
+ </button>
+ <span class="page-topbar__brand" aria-hidden="true">
</code_context>
<issue_to_address>
**issue (bug_risk):** The new top-bar Back button is not included in the interaction controller's `backButton` selector, which only selects `#back-button-bottom` or `.back-button`. No handler is attached to `#back-button-top`, so clicking the visible top Back control does nothing.
**Triggers:** When the user clicks the top-bar Back button.
**Suggested fix:** Select both back buttons, for example `#back-button-top, #back-button-bottom, .back-button`, and attach the navigation handler to each.
</issue_to_address>
### Comment 5
<location path="src/styles/css/cosmic-console.css" line_range="122-134" />
<code_context>
+ );
+}
+
+body.cosmic-console .page-shell .welcome-container {
+ clip-path: polygon(
+ 0 0,
+ calc(100% - 24px) 0,
+ 100% 24px,
+ 100% calc(100% - 24px),
+ calc(100% - 24px) 100%,
+ 0 100%,
+ 0 24px,
+ 24px 0
+ );
+}
+body.cosmic-console .page-shell .welcome-container::after {
+ content: "";
+ position: absolute;
+ inset: 1px;
+ clip-path: polygon(
+ 0 0,
+ calc(100% - 22px) 0,
+ 100% 22px,
+ 100% calc(100% - 22px),
+ calc(100% - 22px) 100%,
+ 0 100%,
+ 0 22px,
+ 22px 0
+ );
+ border: 1px solid var(--cosmic-signal);
+ pointer-events: none;
+ z-index: 1;
+}
+
</code_context>
<issue_to_address>
**nitpick (bug_risk):** The cosmic chamfer and border overlay target `.welcome-container`, but the redesigned welcome page no longer contains an element with that class. The intended chamfered welcome panel and its cosmic border are therefore never rendered.
**Triggers:** When cosmic-console visuals are enabled on the welcome page.
**Suggested fix:** Retarget these rules to the redesigned welcome panel, such as `.welcome-stage`, or retain the `.welcome-container` class on the relevant element.
```suggestion
body.cosmic-console .page-shell .welcome-stage {
clip-path: polygon(
0 0,
calc(100% - 24px) 0,
100% 24px,
100% calc(100% - 24px),
calc(100% - 24px) 100%,
0 100%,
0 24px,
24px 0
);
}
body.cosmic-console .page-shell .welcome-stage::after {
```
</issue_to_address>
### Comment 6
<location path="src/scripts/welcome-page.settings.js" line_range="124-128" />
<code_context>
+ elements.highContrastInput.checked = Boolean(settings.display?.highContrast);
+ }
+ if (elements.uiScaleInput) {
+ elements.uiScaleInput.value = String(Number(settings.display?.uiScale) * 100 || 100);
+ }
+ if (elements.volumeInput) {
+ elements.volumeInput.value = String(Number(settings.sound?.volume) || 80);
</code_context>
<issue_to_address>
**issue (bug_risk):** The range controls are populated with `Number(value) || fallback`, so a persisted volume of `0` is displayed as `80` and a persisted UI scale of `0` is treated as `100%`. The volume zero value is valid according to the settings range and is silently misrepresented in the dialog.
**Triggers:** When the saved master volume is 0.
**Suggested fix:** Use an explicit finite-value or nullish check instead of `||`, such as `Number.isFinite(Number(value)) ? value : 80`.
```suggestion
elements.uiScaleInput.value = String(Number.isFinite(Number(settings.display?.uiScale)) ? Number(settings.display?.uiScale) * 100 : 100);
}
if (elements.volumeInput) {
elements.volumeInput.value = String(Number.isFinite(Number(settings.sound?.volume)) ? Number(settings.sound?.volume) : 80);
}
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and this changes the page layout and interaction runtime across multiple surfaces, adds a vendored external submodule, and changes service-worker caching, so a defect could affect users until the next normal fix or cache update. User settings and cached assets can outlive a revert, but both are bounded and repairable by resetting settings or clearing/updating the cache.
Blocking findings: src/pages/game.html:374, src/pages/level-select.html:240, src/scripts/level-select-page.settings.js:13, src/pages/level-select.html:44, src/scripts/welcome-page.settings.js:128
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Summary by Sourcery
Harden the UI with shared responsive layouts, richer settings, optional cosmic visuals, and improved helper feedback.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: